Repository navigation
live HR banner: read what it shows once the change has landed - #2437
UtkuDenizAltiok wants to merge 1 commit into
Conversation
A tester took the strap off at 10:48 (24 Sep 2026, build with ryanbr#2422). The strap log reads "Strap: WRIST_OFF; live heart rate cleared" at 10:48:03, so the strap said it at once and NOOP cleared the heart rate, but no "Live HR banner: –" line follows: the banner kept the last number until iOS's stale date drew the dash, two to three minutes later. The same at 10:53:39, when the zeros of the strap going back on cleared it. The banner shows AppModel's smoothed rate (`bpm`), and it refreshed from a sink of its own on the heart rate. AppModel resets that median in a sink of its own on the same publisher; a `@Published` sink runs in willSet, and several sinks on one publisher run in no promised order. So the banner could read the median before AppModel had reset it, see a number where there was none, and push nothing. The earlier comment claiming AppModel's sinks run first was wrong. The banner is now refreshed once the changes have landed (`LiveHRBannerInputs.settled`): the heart rate, the link and the median are merged and debounced to the end of the main queue's current turn, where each reads as it now is. A clear moves several of them in one turn and makes one refresh instead of one per value. Nothing else changes: what the banner shows, when it starts, ends and renews. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sh fix PR ryanbr#2437; build 3ad319d; main mirrored to 141cbd9
…fix; ryanbr#2437 description updated; hr-timeline run detection fixed
The rationale landed with a mechanism that does not hold. AppModel subscribes its own sinks in `init()` and `follow()` runs later, from `StrandiOSApp`, so the delivery order is fixed, and `ingestHR(nil, [])` has already taken the median through `resetSmoothing()` by the time the banner's sink runs. Sink ordering was never the hazard. What the old shape missed is `bpm`, which it never observed at all. The median moves on the R-R alone: `clearBiometrics()` clears the heart rate while the R-R is still there, `ingestHR` keeps the median from `rr.last`, and the one refresh the banner gets still reads a live number; `rr.removeAll()` then resets the median with nothing the banner watches left to move. The test also pins the delivery queue, since `refreshBanner` reads UIKit. Refs #2437
|
Merged via #2481, rebased so your commit keeps its authorship: a35d102, with 0efc747 on top. Thanks @UtkuDenizAltiok. Adding One correction went in on top. The rationale attributed the bug to Combine sink ordering, and that does not hold here: The real hole is the one your Also added a main-queue assertion to the first test, since |
…2437 ryanbr#2444), ryanbr#2446 fixed upstream; logs+backup analysed (BACKLOG A background re-score, B ryanbr#2371 answered); fork tidied; build 0ad5ba9 shipped
What this PR does
A follow-up to #2422, found on the tester's phone the morning after it merged. He took the strap off at 10:48. The strap log reads "Strap: WRIST_OFF; live heart rate cleared" at 10:48:03, so the strap said it within about a second and NOOP cleared the heart rate, but no "Live HR banner: –" line follows: the banner kept his last number until iOS's stale date drew the dash, two to three minutes later. The same happened at 10:53:39, when the zeros of the strap going back on cleared it.
Why. The banner shows AppModel's smoothed rate (
bpm), and it refreshed from a sink of its own onlive.$heartRate. AppModel resets that median in a sink of its own on the same publisher. A@Publishedsink runs in willSet, and several sinks on one publisher run in no promised order, so the banner could read the median before AppModel had reset it, see a number where there was none, and push nothing. #2422 carried a comment saying AppModel's sinks run first; that was wrong, and this removes it.The fix. The banner is refreshed once the changes have landed:
LiveHRBannerInputs.settledmerges the heart rate, the link and the median, and debounces them to the end of the main queue's current turn, where each reads as it now is. A clear moves several of them in one turn and makes one refresh instead of one per value; a worn strap still makes one refresh per heart-rate tick, as before. What the banner shows, and when it starts, ends and renews, is unchanged.The rest of the tester's run matched #2422: a dropped link showed the dash at once and the number came back a second after the reconnect (11:09:57 → 11:17:16); with NOOP swiped away the banner stayed, iOS drew the dash about two minutes later, and opening NOOP picked it up with the number within a second; the switch ended it and started it again at once. The strap now names WRIST_OFF in the log, so the stale-date question from the #2422 review can be answered from the next log, once the dash comes from WRIST_OFF.
Type of change
How it was tested
LiveHRBannerInputsTests(2, StrandTests): a cleared heart rate is read only once the clear has landed, and reads nil; a clear and a dropped link in one turn make one refresh. Seen to fail with the debounce removed: the reader saw[91, 91](the old value, read inside the clear) and 6 refreshes instead of 1. Restored byte-identical (sha256).bc5da07d, on141cbd93), every step passed: WhoopStore 611 · StrandAnalytics 2056 · StrandImport 327 · doc lint · i18n · parity ledger, ratchet, governance (124) · macOSStrandTests2,144 (only the two locale-dependentTodayCarryOverTests) · iOS build.Checklist
docs/CONTRIBUTING.mdStrand.xcodeproj/) or any secrets/keystores🤖 Generated with Claude Code